Repository navigation
feat(blueprint): resolve data_dir auto to the largest data disk on fresh accounts - #1343
Merged
Merged
Conversation
…ta_dir.yml` in preflight.yml did not carry the `always` tag into the included tasks. Tag-limited runs skipped the resolution and left `data_dir` as the literal 'auto', which produced `findmnt -T auto`. preflight.yml now uses `import_tasks`, so the tasks inherit the tag and the `when` still gates each one. The earlier fixes stay: the /etc mount exclusion for the worker image build and the corrected preflight slice index in test_ansible_deprecations.py. Local run of test_removed_backend.py, test_ansible_deprecations.py and test_configuration.py: 207 passed, 1 skipped. I did not run the real smoke job or the Docker build
…r the image build; I have no Docker access here. A final run of `tests/test_configuration.py` and `tests/test_chart.py` hit my 280 s limit, so I have no result for it. The two files that cover the runtime fixture, `tests/test_removed_backend.py` and `tests/test_fleet_units.py`, passed: 10 passed, 1 skipped. 1. Runtime smoke (`project database runtime smoke`): the fixture's `ship.provision` document had no `data_dir`. The default `auto` therefore reached the storage guard unresolved and ran `findmnt -T auto`. I pinned `"data_dir": ""` in `tests/test_removed_backend.py`. Both apply paths now set `data_dir` to the empty string. The other `apply()` callers in `tests/test_fleet_units.py` already set `data_dir`. 2. Worker image smoke: I took the review advice and pinned `data_dir: ""` in `blueprint/containers/crewship.container.yml`. An image build should not probe host disks, and `auto` could pick a build mount. I have not confirmed that `auto` caused the missing `chrome-devtools-mcp.js`. The npm 1.13.0 entrypoint move, which my earlier check found, may be the cause; the image build must show it. I did not change the hardcoded `build/src/bin` path, and the image smoke may still fail on it. The 'configuration and python checks' job failed only because it needs the runtime smoke job to pass
…3.0 moved its bin entrypoint to `out/Default/gen/src/bin/chrome-devtools-mcp.js`. The package is installed unpinned, so the old `build/src/bin` path was stale. I updated `crewship_chrome_devtools_mcp_path` in `blueprint/ansible/group_vars/all/base.yml` and the two path sites in `tests/test_configuration.py`. `uv run pytest -q tests/test_configuration.py -k "devtools or board_environment"` passes (2 tests), including the test the earlier run missed. `tests/container-smoke.sh` has no `build/` path. I did not run the Docker image build or the smoke job. The path is still hardcoded, so a later upstream layout move would break it again
i098
marked this pull request as ready for review
November 12, 2026 02:13
i098
commented
Nov 12, 2026
i098
left a comment
i098
left a comment
Owner
Author
There was a problem hiding this comment.
Validation review at head 30ffb25: the pipeline checked the data-disk picker, config schema and docs. It found the Ansible test copy list missing the new task file; fixed. The live host move is proven in the task report and not rerun. Full suite fails under host load on base too. 18 of 18 checks pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1339
Summary
The default
crewship.data_diris nowauto. On a fresh account it resolves to the largest mounted data disk of 100 GiB or more, so worktree pools and tool caches start off the system disk.What changed and why
blueprint/ansible/tasks/data_dir.yml, whichpreflight.ymlincludes whendata_dir == 'auto'. It picks the largest mounted ext4, xfs or btrfs disk of 100 GiB or more. It excludes/,/boot,/snap,/usr,/var,/home,/media,/runand/tmp.~/.treehouseon the legacy layout, so existing pools and Docker data never move.blueprint/config/default.ymlfrom""toauto, and let the schema pattern increwship.schema.jsonacceptauto.data_dir: ""still opts out, andautowithout a qualifying disk behaves like"".docs/configuration.mdanddocs/features.md(the data disk is no longer opt-in), and add the changelog fragment.tests/test_chart.pyandtests/test_configuration.py.Test evidence
data_dir.ymlrun with ansible-playbook on localhost against fake mounts: largest disk chosen, excluded mounts skipped, recorded disk kept, unmounted disk fails.Checklist
fix(provisions): check the digest before install.uv run pytest). Not applicable: the touched files pass; the full suite has failures that also occur on the base commit, so CI decides.uv run ruff check).--syntax-check).launch→launch=changed=0on the second run. Not applicable: no host launch was run; CI covers it.changelog.d/<issue>.<type>.md(not aCHANGELOG.mdedit), or theno changeloglabel.Validation
Risk Assessment
✅ Low: The persistence fix, the narrowed mount filter and the real-playbook test from the earlier round hold. The advisory's two fail-open paths are real but narrow, and I judged neither a defect in intended usage.
Testing
I ran the two touched test files under the
cf-sandboxwrapper and under plainuv run. Both runs passed. These are unit-level and synthetic-mount tests, not live runs of the product. The full-suite exit 1 is unexplained. The host move was not driven, so free space before and after is not measured.auto, an empty string and an absolute path for data_dirPipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed ✅
blueprint/ansible/tasks/preflight.yml:17-autois resolved fromansible_facts.mountson every apply, and nothing stores the result. This gives three sequences where a later apply resolves differently from the first one. (1) The first apply resolvesautoto the data disk. Treehouse then runs withoutTREEHOUSE_ROOT, for example from a non-login shell, and creates~/.treehouse. The next apply resolvesautoto''. The account.yml branches for an emptydata_dir(lines 328-345) then remove the managed bulk-defaults file and the.bashrcblock. Docker data root and syslog setup also stop applying. (2) The data disk is unmounted, or not yet mounted, during an apply.autothen resolves to''with no error, and docs/configuration.md:409 says that clearingdata_dirremoves the managed Bash block and user-manager file. TMPDIR and the cache env vars silently return to the system disk. (3) A larger qualifying disk is added later.automoves to it, socrewship_docker_data_rootand the env.d values change. The Treehouserootin config.toml stays put, because bulk-defaults only adds missing keys. The~/.treehouselegacy check does not stop (2) or (3). Pools under<data_dir>/.treehousenever create~/.treehouse. This breaks the site.yml rule that a second unchanged apply reports changed=0. The smallest remedy is to persist the first resolution. For example, treat an existing managed bulk-defaults file or<data>/.treehouseas the choice already made, and fail instead of falling back to''when that disk is missing. That remedy adds durable state, so it needs author authorization.blueprint/ansible/tasks/preflight.yml:25- The mount filter excludes only/boot,/snap,/usr,/varand/. A separate large/homepartition or a removable/media/...or/run/media/...disk of 100 GiB or more can be picked silently. With/homechosen,data_dirbecomes/home, so Docker's data root becomes~. With a removable disk chosen,/tmpand the cache env vars point at it. The new default therefore changes where Docker, syslog and caches live on every fresh host without opt-in. The intent only asks for Treehouse pools and the bun, pnpm and uv caches on the data disk. The narrower form is to keep the default""and have the host set an explicit path.tests/test_configuration.py:265- The new test only checks that the schema accepts the three values. Nothing runs the resolution logic in preflight.yml. That logic covers the legacy-pool skip, the choice of the largest qualifying mount, the exclusions, and the empty fallback. A wrong filter or sort would pass the tests. test_chart.py only pinsdata_dirto""to avoid the new default, so it also does not exerciseauto. Feed a syntheticansible_facts.mountsthrough the real playbook or a task fixture and assert the resultingdata_dir.🔧 Fix applied.
✅ Re-checked - no issues remain.
tests/- The configured full-suite command exited 1 before this phase. I could not reproduce or name that failure. The two files this change touches pass in full: tests/test_chart.py (47 passed, 18 skipped) and tests/test_configuration.py (192 passed). A full-suite rerun did not finish in over 10 minutes and I cancelled it. The failure is probably in an unrelated or timing-dependent test, for example a slow test hitting the worker timeout. Remote CI should confirm.blueprint/ansible/tasks/data_dir.yml- The host move itself was not driven: moving the Treehouse pools and the bun/pnpm/uv caches, the before/after free-space numbers, and the live-worker and unpushed-work guards. This is an operator action on real host data, outside what this phase may touch. Only the Crewship recipe change was exercised.cf-sandbox -- uv run pytest -n 4 --dist worksteal testscf-sandbox -- uv run pytest -q -k data_dir tests/test_configuration.py(13 passed; the resolution cases run real ansible-playbook)cf-sandbox -- uv run pytest -q -x tests/test_chart.py(47 passed, 18 skipped)cf-sandbox -- uv run pytest -q -n 4 tests/test_configuration.py(192 passed)grep of tests/ for data_dir, TMPDIR and bulk assumptions; other suites pass explicit data_dir valuescf-sandbox -- uv run pytest -n 4 --dist worksteal testsrerun, cancelled after more than 10 minutes with no result🔧 Fix applied.
3 issues (1 error, 2 warnings) still open:
cf-sandbox -- uv run pytest -n 4 --dist worksteal testsexited 1 before this phase. I could not reproduce it. Undercf-sandboxI ranuv run pytest -q tests/test_configuration.py tests/test_chart.py -n 4: 239 passed and 18 skipped. The 18 skips are the sudo-blocked cases, andtest_auto_data_dir_resolutionpassed inside the sandbox. A full-suite rerun earlier did not finish in over 10 minutes and I cancelled it. The failure is probably in a test outside these two files, or a timing-dependent one. Remote CI should confirm.auto, an empty string and an absolute path for data_dir, Move the Treehouse pools and the bun/pnpm/uv caches to the data disk, with free space before and after, and never touch live or unpushed poolsauto, an empty string and an absolute path for data_dircf-sandbox -- uv run pytest -n 4 --dist worksteal testscf-sandbox -- uv run pytest -q tests/test_configuration.py tests/test_chart.py -n 4(239 passed, 18 skipped)uv run pytest -q tests/test_configuration.py tests/test_chart.py -n 4(257 passed)test_auto_data_dir_resolutionruns the realdata_dir.ymlthroughansible-playbookon a synthetic mount list. It passed in both environments.test_data_dir_accepts_auto_empty_or_a_pathrunsship.validate_configonauto, an empty string and a path.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.